Skip to content

Fixed: Sanitize fetched page text in HttpViewHandler#1294

Open
mridulpathak wants to merge 1 commit into
apache:trunkfrom
mridulpathak:sanitize-page-text-in-HttpViewHandler
Open

Fixed: Sanitize fetched page text in HttpViewHandler#1294
mridulpathak wants to merge 1 commit into
apache:trunkfrom
mridulpathak:sanitize-page-text-in-HttpViewHandler

Conversation

@mridulpathak

Copy link
Copy Markdown
Contributor

Fixed: Sanitize fetched page text in HttpViewHandler

@mridulpathak
mridulpathak force-pushed the sanitize-page-text-in-HttpViewHandler branch 2 times, most recently from cc99a1d to e0e94e9 Compare May 28, 2026 15:43
@mridulpathak
mridulpathak force-pushed the sanitize-page-text-in-HttpViewHandler branch from e0e94e9 to f3a7071 Compare May 28, 2026 16:03
@jacopoc

jacopoc commented May 29, 2026

Copy link
Copy Markdown
Contributor

@mridulpathak My guess, after reviewing the changes in this pull request, is that you are trying to address some "code quality" alerts reported by a static analysis tool (such as CodeQL). Is my guess correct?

If that is the case, my suggestion would be to postpone the decision to merge it, even if it resolves the alerts, provided that the changes do not actually change the behavior of the system (i.e., they are functionally equivalent to the current version) and only make the code easier for static analysis tools to assess.

I suggest this because I have found a few other alerts that may be false positives and may also be resolved by simplifying the code path. For now, I have decided not to merge changes that are functionally equivalent to the current code solely to resolve alerts, and instead focus only on resolving alerts that require functional changes to the codebase.

Once all valid alerts are resolved, we can reconsider the ones that appear to be false positives and decide how we want to address them (possibly revisiting pull requests like this).

What do you think?

@mridulpathak

Copy link
Copy Markdown
Contributor Author

Sure @jacopoc. It makes complete sense. I'll hold on to this one and focus on identifying and fixing valid alerts. We could revisit these false positives later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants